Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port over the existing tests and set up CI #11

Merged
merged 108 commits into from
Jul 12, 2024
Merged

Conversation

kaitejohnson
Copy link
Collaborator

@kaitejohnson kaitejohnson commented Jul 8, 2024

This PR moves over the existing testing infrastructure from https://github.com/CDCgov/wastewater-informed-covid-forecasting, and modifies the tests as needed to fit the new functions. It also sets up CI to run the tests every time there is a PR into main or prod. Other scope creep things it does:

  • adds documentation to the data using data.R
  • includes some but not complete set of examples for exported functions

Definitely need to figure out how to set up caching because the r-cmd-check is taking on the order of 20 min to fail :( I think this can be a separate issue though.

Do we want to set up any other CI? For example, I know @zsusswein set up code-cov (we would not do well right now....) and also a check to make sure that the change log( NEWS.md) was updated every time a PR merges to main

Need to have main merged in once #5 is merged

@kaitejohnson kaitejohnson marked this pull request as ready for review July 11, 2024 14:02
@kaitejohnson
Copy link
Collaborator Author

Fixing up a few things but think this is almost ready @dylanhmorris however, I don't the pkgdown is working correctly. When I go to Settings >Pages it says that it was deployed 2 weeks ago, and I think it should have been deployed with the latest merge to main?

Edit: added main to pkgdown.yaml, and presuming this should fix it. This is ready for review. Note I moved the COVID delay distributions to delay_distribs.R to keep them as internal package functions.

R/postprocess.R Outdated Show resolved Hide resolved
@kaitejohnson
Copy link
Collaborator Author

@dylanhmorris this is ready for re-review

Copy link
Collaborator

@dylanhmorris dylanhmorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things

.github/workflows/r-cmd-check.yaml Outdated Show resolved Hide resolved
DESCRIPTION Outdated Show resolved Hide resolved
DESCRIPTION Outdated Show resolved Hide resolved
DESCRIPTION Outdated Show resolved Hide resolved
data-raw/covid_pmfs.R Outdated Show resolved Hide resolved
R/get_stan_data.R Outdated Show resolved Hide resolved
R/wwinference.R Show resolved Hide resolved
inst/stan/simplewwinference.stan Outdated Show resolved Hide resolved
Copy link
Collaborator

@dylanhmorris dylanhmorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor wording suggestion

R/wwinference.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@dylanhmorris dylanhmorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provided the CI checks pass this LGTM!

@kaitejohnson kaitejohnson merged commit 371748c into main Jul 12, 2024
4 checks passed
@kaitejohnson kaitejohnson deleted the 9-port-over-tests branch July 12, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants